Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Move Seccomp support to use Libseccomp as backend #633

Closed
wants to merge 4 commits into from

Conversation

mheon
Copy link

@mheon mheon commented Jun 15, 2015

In a brief summary of prior events: there were two competing Seccomp integrations, a native Golang implementation (PR #613) and one based on the libseccomp library (PR #384). The libseccomp PR had licensing issues preventing its merging, so the native Golang implementation was merged.

I believe that the licensing issues present in the old Libseccomp based implementation have been resolved, and that the library-based solution is a decidedly superior implementation to the current Golang version. Libseccomp is well-tested, well-supported, and high-performance, and removes the burden of supporting this code from Docker.

This PR removes the existing Seccomp implementation and replaces it with a Libseccomp based one. The API from the original Libseccomp PR has been substituted for the existing API, but the two are mostly compatible, and it should be possible to convert this to use the existing API if that is desired.

@rhatdan
Copy link
Contributor

rhatdan commented Jun 15, 2015

This would be our preferred solution.

@LK4D4
Copy link
Contributor

LK4D4 commented Jun 15, 2015

@mheon You need to fix gofmt.

@pcmoore
Copy link

pcmoore commented Jun 15, 2015

I feel there are several advantages to the libseccomp approach versus the pure golang approach currently adopted by Docker, some of the highlights are listed below:

  • Performance: a lot of thought and work has gone into the libseccomp library to optimize the generated BPF filter code in an effort to minimize the overhead at each syscall invocation; the pure golang approach does not appear to have made any optimizations.
  • Correctness: There are several bugs/issues with the pure golang implementation that have been mentioned and not addressed, or misunderstood, by the developer, e.g. cross platform support, x86_64/x32 interactions, OABI_SYSCALL_BASE, etc.
  • Ongoing maintenance: the libseccomp library receives ongoing support and updates to the internal syscall tables, the golang bindings inherit this support.

@LK4D4
Copy link
Contributor

LK4D4 commented Jun 15, 2015

@mheon There are some vet warnings in your library.

@mrunalp
Copy link
Contributor

mrunalp commented Jun 15, 2015

@LK4D4 One of them seems to be in protobuf :/

vendor/src/github.com/golang/protobuf/proto/all_test.go:1284: result of fmt.Sprintf call not used
exit status 1
vendor/src/github.com/seccomp/libseccomp-golang/seccomp_test.go:287: result of fmt.Errorf call not used
exit status 1

@LK4D4
Copy link
Contributor

LK4D4 commented Jun 15, 2015

I wonder how it worked before.

@LK4D4
Copy link
Contributor

LK4D4 commented Jun 15, 2015

Looks like something with update-vendor script, because your PR also downgraded gocapability. Probably @avagin did some magic in his vendor protobuf commit.

@mheon
Copy link
Author

mheon commented Jun 15, 2015

Looks like I must have messed it up when rebasing... I'll take a look.

The error in the libseccomp bindings looks like a typo, so that should be easy to fix as well.

@LK4D4
Copy link
Contributor

LK4D4 commented Jun 17, 2015

@mheon We fixed some stuff in master. You can try rebase.

@mheon
Copy link
Author

mheon commented Jun 17, 2015

@LK4D4 Rebased.

The static analysis problem in the library bindings should still be there, waiting for a patch for that to make its way upstream. Hopefully will be fixed by end of day.

@mheon
Copy link
Author

mheon commented Jun 18, 2015

Alright, build failures are resolved.

@glevand
Copy link
Contributor

glevand commented Jun 25, 2015

libseccomp-2.2.0 and newer have arm64 support (2.2.1 is specified in this Dockerfile), so this PR should resolve the arm64 build breakage I reported in https://github.com/docker/libcontainer/issues/636.

mheon added 4 commits June 26, 2015 11:47


This PR introduces the ability to filter system calls on a per-container basis on Linux, using libseccomp to support multiple architectures.

This adds another layer of security between containers and the kernel. System calls which are unnecessary in a container or problematic from a security perspective can be restricted to prevent their use. Most of the truly problematic syscalls are already restricted by dropping capabilities; this adds an additional, finer-grained layer of protection.

This PR adds a vendored library dependency (Go bindings for libseccomp) and a build dependency on libseccomp >= v2.1. The actual changes to libcontainer are fairly minimal, most of the delta is in the libseccomp bindings.

Docker-DCO-1.1-Signed-off-by: Dan Walsh <[email protected]> (github: rhatdan)
Docker-DCO-1.1-Signed-off-by: Matt Heon <[email protected]> (github: mheon)
Signed-off-by: Matthew Heon <[email protected]>
This should also undo accidental gocapability downgrade

Signed-off-by: Matthew Heon <[email protected]>
@mheon
Copy link
Author

mheon commented Jun 26, 2015

Rebased on latest master.

@LK4D4
Copy link
Contributor

LK4D4 commented Jun 26, 2015

@mheon Could you port your PR to opencontainers/runc pls?

@mheon
Copy link
Author

mheon commented Jun 26, 2015

@LK4D4 Sure, I'll look into getting it ported.

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 14, 2015

ported to runc

@LK4D4 LK4D4 closed this Jul 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants